Conversation
…e to remind myself.
… in the "textVersion" when we do the next official release.
R/construct_api_requests.R
Outdated
There was a problem hiding this comment.
Does this need to be changed to date?
There was a problem hiding this comment.
Actually, I think it should be changed to logical (because it tells the code whether or not to return a date or dateTime)
There was a problem hiding this comment.
Oh, I meant the input is named "date" and it is logical, right?
|
|
||
| if(length(datetime) == 1){ | ||
| # If the user has "P" or the "/" we assume they know what they are doing | ||
| if(grepl("P", datetime, ignore.case = TRUE) | |
There was a problem hiding this comment.
FYI, I tried this with a lowercase "p7d" and "p7D" and the API will not have it. I don't think there's necessarily anything you need to do here, just a random test.
There was a problem hiding this comment.
we can plop a toupper in there, good to know!
ehinman
left a comment
There was a problem hiding this comment.
This looks good. Approved. I particularly love this implementation:
start_end <- as.POSIXct(c("2021-01-01 12:15:00",
"2022-01-01 16:45"),
tz = "America/New_York")
dataRetrieval::format_api_dates(start_end)
Very nice.
A couple thoughts/suggestions that are optional: The function documentation for time, begin, and end don't go into detail on the range of dates that the functions will accept. Is this intentional?
In the Water Data APIs vignette, it does a few things with the time input, and there's a section that covers the time zone reference table, but it might be nice to show off using UTC offsets in an example, or the really lovely time zone example I highlight above, where you don't have to futz with offsets at all.
I added a Detail section: Details Let me know if you have any suggestions to fix that. |
| #' @param approval_status `r get_ogc_params("daily")$approval_status` | ||
| #' @param last_modified `r get_ogc_params("daily")$last_modified` | ||
| #' | ||
| #' See also Details below for more information. |
There was a problem hiding this comment.
These don't all have Details, right?
There was a problem hiding this comment.
Line 49 below:
#' @inherit read_waterdata_continuous details
Brings the same "Detail" paragraph into other functions' help page
There was a problem hiding this comment.
Ahhhh ok, neat. Thanks for sharing that.
R/read_waterdata_continuous.R
Outdated
| #' the timeseries until 2024-01-01. | ||
| #' By default, time is assumed UTC, although time zone attributes | ||
| #' will be accommodated. As an example, setting \code{time = as.POSIXct(c("2021-01-01 12:00:00", | ||
| #' "2021-01-01 12:00"), tz = "America/New_York")} will request data that between |
NEWS
Outdated
| * Added tests for read_waterdata_stats functions | ||
| * Updated next_req_url to allow paging through /statistics API output | ||
| * Added deprecate message to readNWISstat function. | ||
| * Added deprecate message to readNWISstat function |
There was a problem hiding this comment.
"deprecation"? I don't know why github isn't letting me direct edit things now. Maybe because I approved?
|
Details looks good besides small typo that I made a comment about. |
Should now allow this:
(which fixes #853 )
and this:
(which fixes #839 )